Review

+188 / -11 across 3 files — adds USDSUI single-asset looping support with reward collection and multi-hop swapping via Bluefin DEX.

Issues

1. Swap direction likely inverted for USDC → USDSUI (critical)

In collectAndSwapRewards(), the final swap step for USDSUI pools has:

typeArguments: [this.poolLabel.asset.type, this.poolLabel.asset.type, usdcCoin.coinType],

This puts USDSUI as both the pool asset type AND the "from" coin, with USDC as the "to" coin. But the intent is to swap USDC → USDSUI. The getPoolIdBySymbolsAndProtocol call correctly uses ('USDC', 'USDSUI', 'bluefin'), but the type arguments and tx.pure.bool(false) for a_to_b look contradictory. Please verify against the Move contract's expected type param order — a wrong swap direction would lose user funds.

2. Hardcoded slippage tolerances with no documentation

Multiple magic numbers: 1000, 10, 10000. No comments explaining:

• What unit they're in (basis points? DEX-specific?)
• Why ALPHA→stSUI uses 1000, stSUI→SUI uses 10, and everything else uses 10000
• If 10000 = 100% slippage, that's significant MEV risk

3. Reward coin list is hardcoded and not pool-aware

collectAndSwapRewards() fetches 10 specific coin types regardless of which rewards the pool actually earns. This means unnecessary RPC calls, manual updates when new reward tokens are added, and silent skipping of unknown reward types.

4. No error handling for missing Bluefin pools

getPoolIdBySymbolsAndProtocol / getPoolIdByTypesAndProtocol are awaited inside a loop with no handling if a Bluefin pool doesn't exist for a reward pair. This would throw and abort the entire deposit/withdraw transaction.

5. let should be const

let coinTypes = [...] is never reassigned — should be const.

6. Test file left in dry-run mode

testRun.ts has executeTransactionBlock commented out. Fine for testing, but worth calling out so it doesn't surprise anyone.

7. Empty PR description

No context on what USDSUI single-asset looping is, why reward collection happens during deposit/withdraw, or how to test.

Verdict

The core logic follows the existing SingleAssetLooping pattern and looks structurally sound. However, the USDC→USDSUI swap type arguments need verification against the Move contract before merging. The slippage values also need documentation or a confirming comment that 10000 is intentional and safe.

Re-reviewed after Shrome's changes and clarifications.

Critical concern from prior review resolved by explanation:

• The 10000 / 1000 / 10 values are minimum-swap-amount, not slippage bps — no MEV risk as I'd framed it.
• For the USDC→USDSUI hop, type args [USDSUI, USDSUI, USDC] with a_to_b: false correctly swaps USDC→USDSUI per the Move signature [pool_asset, A, B].
• let → const done.
• collectAndSwapRewards now also called from claimWithdraw and cancelWithdraw — good catch.

Remaining nice-to-haves (non-blocking, follow-up welcome):

• Add a short comment documenting that the u64 arg is min-swap-amount (and the unit), so the next reader doesn't repeat my misread.
• Pool-aware reward list and graceful handling of missing Bluefin pools can be revisited later.

LGTM.